Skip to content

Conversation

@xylar
Copy link
Collaborator

@xylar xylar commented Nov 12, 2025

To accomplish this, this PR moves Omega <--> MPAS-Ocean variable translation to the Ocean component class. It removes the unneeded Omega --> MPAS-Ocean dictionaries (we can use the MPAS-Ocean --> Omega dictionaries for both direction). It also adds methods for translating a list of variable names, rather than a dataset (useful for validation variables).

This approach prevents redundant building of the translation maps for each step that needs them and makes the translation available beyond OceanIOSteps.

OceanIOSteps has been updated to use the methods from Ocean component for mapping variable and dimension names.

Checklist

  • API documentation in the Developer's Guide (api.md) has any new or modified class, method and/or functions listed
  • Documentation has been built locally and changes look as expected
  • Testing comment in the PR documents testing used to verify the changes

Fixes #395

@xylar xylar added bug Something isn't working ocean Related to the ocean component labels Nov 12, 2025
@xylar
Copy link
Collaborator Author

xylar commented Nov 12, 2025

Testing

After struggling to test on Perlmutter, I switched to Chrysalis (with Intel), where I was able to successfully run both the omega_pr and MPAS-Ocean pr suites, including baseline comparisons. All tests passed -- baseline comparisons passed as expected (given the baseline and comparison Polaris and ocean components are the same).

@xylar
Copy link
Collaborator Author

xylar commented Nov 12, 2025

@cbegeman, could you give this a quick look when you can? I don't think more testing is needed than what I already did.

@xylar xylar requested a review from cbegeman November 12, 2025 10:43
@xylar xylar mentioned this pull request Nov 12, 2025
1 task
Copy link
Collaborator

@cbegeman cbegeman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like this change! My only question would be whether we should do the component is ocean check in ocean_io_step's __init__ rather than its methods.

@xylar
Copy link
Collaborator Author

xylar commented Nov 12, 2025

My only question would be whether we should do the component is ocean check in ocean_io_step's init rather than its methods.

I'll look into that but I think probably so.

@xylar
Copy link
Collaborator Author

xylar commented Nov 12, 2025

There seems to be an easier fix that I'll push in just a second.

This merge removes unneeded Omega --> MPAS-Ocean dictionaries.
It also adds methods for translating a list of variable names,
rather than a dataset (useful for valiation variables).

This approach will prevent redundant building of the translation
maps for each step that needs them and will make the translation
available beyond OceanIOSteps
...in OceanIoStep and OceanModelStep
@xylar xylar force-pushed the fix-omega-pr-baseline-comparison branch from 6380088 to e221c6e Compare November 12, 2025 16:59
@xylar
Copy link
Collaborator Author

xylar commented Nov 12, 2025

@cbegeman, 07da14a seems like an elegant way to take care of this. I'm going to go ahead and merge once CI finishes but I'll test this further in my next branches I'm working on.

@xylar xylar merged commit 37d8818 into E3SM-Project:main Nov 12, 2025
6 checks passed
@xylar xylar deleted the fix-omega-pr-baseline-comparison branch November 12, 2025 17:07
@cbegeman
Copy link
Collaborator

@xylar Yes, that looks great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ocean Related to the ocean component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

baseline comparisons failing for Omega tests

2 participants